Skip to content

Deny unknown fields on RPC Request and Response#6926

Draft
sudo-shashank wants to merge 10 commits intomainfrom
shashank/deny-unknown-fields-rpc
Draft

Deny unknown fields on RPC Request and Response#6926
sudo-shashank wants to merge 10 commits intomainfrom
shashank/deny-unknown-fields-rpc

Conversation

@sudo-shashank
Copy link
Copy Markdown
Contributor

@sudo-shashank sudo-shashank commented Apr 16, 2026

Summary of changes

Changes introduced in this pull request:

  • Reject unknown fields in RPC request parameters and response payloads when FOREST_STRICT_JSON is enabled.
  • There are a few mismatches caused by unknown fields in some RPC responses, so FOREST_STRICT_JSON is currently not enabled in CI.

Reference issue to close (if applicable)

Closes #5600

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

  • New Features

    • Enhanced strict JSON validation mode now rejects unknown fields in RPC requests and responses when FOREST_STRICT_JSON is enabled.
  • Documentation

    • Updated FOREST_STRICT_JSON environment variable documentation to reflect new unknown-field rejection behavior.
  • Tests

    • Extended test coverage for strict JSON validation mode, including unknown-field handling scenarios.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 16, 2026

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: denying unknown fields on RPC requests and responses when strict JSON validation is enabled.
Linked Issues check ✅ Passed The PR implements both #5600 (deny unknown fields in RPC requests) and #5635 (deny unknown fields in RPC responses) by adding a validator function that rejects unknown fields when FOREST_STRICT_JSON is enabled.
Out of Scope Changes check ✅ Passed All changes are directly related to denying unknown fields in RPC requests/responses and updating related documentation, tests, and configurations.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch shashank/deny-unknown-fields-rpc
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch shashank/deny-unknown-fields-rpc

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Line 32: Update the CHANGELOG entry that currently references the PR number
[`#6926`] to reference the tracked issue numbers instead; replace the PR link and
label with the issue reference(s) [`#5600`] and/or [`#5635`] and the corresponding
issue URL(s) while keeping the rest of the description unchanged ("Added strict
JSON validation to deny unknown fields in RPC request parameters and response
results when `FOREST_STRICT_JSON` is enabled."); edit the exact line that
contains the current "- [`#6926`](...): ..." entry so the format matches other
changelog entries (use [`#ISSUE_NO`](link-to-issue): <description>).

In `@src/rpc/reflect/mod.rs`:
- Around line 278-285: The current validation only re-serializes Forest's own
LotusJson before returning; to actually reject unknown fields from remote nodes
update RpcMethodExt::call_raw to validate the incoming JSON payload using
crate::rpc::json_validator::from_value_rejecting_unknown_fields (into the
<Self::Ok as HasLotusJson>::LotusJson type) instead of using plain
serde_json::from_value, so the client.call(...) result is passed through
from_value_rejecting_unknown_fields (honoring FOREST_STRICT_JSON behavior) and
any unknown fields cause an error.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9e9c365b-6589-49ef-96e3-d17f624df0ce

📥 Commits

Reviewing files that changed from the base of the PR and between 09376b7 and b48816d.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • CHANGELOG.md
  • Cargo.toml
  • docs/docs/users/reference/env_variables.md
  • src/rpc/json_validator.rs
  • src/rpc/reflect/mod.rs
  • src/rpc/reflect/parser.rs

Comment thread CHANGELOG.md
Comment thread src/rpc/reflect/mod.rs Outdated
@sudo-shashank sudo-shashank added the RPC requires calibnet RPC checks to run on CI label Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RPC requires calibnet RPC checks to run on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deny unknown fields on RPC request deserialization

1 participant